-
Notifications
You must be signed in to change notification settings - Fork 4
[feat] 가게 정보 편집 페이지 구현 #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for thejulge1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
cozy-ito
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 👍
src/pages/ShopEditPage.tsx
Outdated
| import { useUserStore } from "@/hooks/useUserStore"; | ||
| import { extractDigits, numberCommaFormatter } from "@/utils/number"; | ||
|
|
||
| const CATEGORY_OPTIONS = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ShopRegisterPage에도 동일한 상수가 있던데,constants폴더로 분리해서 재사용 하면 어떨까요? 🤔label과value값이 같다면, 아래처럼 작성해도 좋을 것 같아요!
const CATEGORY_OPTIONS = ["한식", "중식", "일식", ... ].map((category) => ({ label: category, value: category }));{ label: " ... ", value: " ... " }를 반복 작성하지 않아도 되니까요! 😄
src/pages/ShopEditPage.tsx
Outdated
| const shop = res.data.item; | ||
| setForm({ | ||
| name: shop.name, | ||
| category: shop.category, | ||
| address1: shop.address1, | ||
| address2: shop.address2, | ||
| originalHourlyPay: numberCommaFormatter(shop.originalHourlyPay), | ||
| description: shop.description, | ||
| }); | ||
| if (shop.imageUrl) setImagePreview(shop.imageUrl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
구조분해할당, 단축 속성을 활용하면 더 코드가 깔끔해 보일 것 같아요! 🤔
| const shop = res.data.item; | |
| setForm({ | |
| name: shop.name, | |
| category: shop.category, | |
| address1: shop.address1, | |
| address2: shop.address2, | |
| originalHourlyPay: numberCommaFormatter(shop.originalHourlyPay), | |
| description: shop.description, | |
| }); | |
| if (shop.imageUrl) setImagePreview(shop.imageUrl); | |
| const { name, category, address1, address2, originHourlyPay, description, imageUrl } | |
| = res.data.item; | |
| setForm({ | |
| name, | |
| category, | |
| address1, | |
| address2, | |
| originalHourlyPay: numberCommaFormatter(originalHourlyPay), | |
| description, | |
| }); | |
| if (imageUrl) setImagePreview(imageUrl); |
| category: "" as ShopCategory, | ||
| address1: "" as SeoulDistrict, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오..! 여기에 as 타입 단언이 들어간 이유가 있나요?!
궁금합니다! 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
초기값으로 빈 문자열을 두면 아래와 같은 오류가 발생하더라구요!
Type '""' is not assignable to type '"한식" | "중식" | "일식" | "양식" | "분식" | "카페" | "편의점" | "기타"'.
ShopCategory는 "한식" | "중식" | "일식" | ...와 같은 리터럴 유니언 타입이라서 "한식"은 괜찮아도 ""는 허용하지 않는 것 같아요. 위와 같이 as 타입 단언을 사용해 지금은 빈 문자열이지만, 나중에 반드시 올바른 ShopCategory 또는 SeoulDistrict 값으로 바뀔 거라고 말해주는 방식을 사용해보았습니다!
src/pages/ShopEditPage.tsx
Outdated
| const reader = new FileReader(); | ||
| reader.onload = () => { | ||
| if (typeof reader.result === "string") { | ||
| setImagePreview(reader.result); | ||
| } | ||
| }; | ||
| reader.readAsDataURL(file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URL.createObjectURL API를 활용한 코드가
위->아래 흐름으로 더 보기 쉬운 것 같아요! 🤔
| const reader = new FileReader(); | |
| reader.onload = () => { | |
| if (typeof reader.result === "string") { | |
| setImagePreview(reader.result); | |
| } | |
| }; | |
| reader.readAsDataURL(file); | |
| // 이미 메모리에 blobURL이 존재한다면 해제하기 | |
| if(imagePreview) { | |
| URL.revokeObjectURL(imagePreview); | |
| } | |
| // blobURL 생성하기 | |
| const blobUrl = URL.createObjectURL(file); | |
| setImagePreview(blobUrl); |
src/pages/ShopEditPage.tsx
Outdated
| try { | ||
| setIsSubmitting(true); | ||
|
|
||
| let imageUrl = imagePreview || ""; | ||
| if (imageFile) { | ||
| const presignedURL = await postImage(imageFile.name); | ||
| await putImage(presignedURL, imageFile); | ||
| imageUrl = getPublicURL(presignedURL); | ||
| } | ||
|
|
||
| const payload = { | ||
| name: form.name.trim(), | ||
| category: form.category, | ||
| address1: form.address1, | ||
| address2: form.address2.trim(), | ||
| originalHourlyPay: hourlyPay, | ||
| description: form.description.trim(), | ||
| imageUrl, | ||
| }; | ||
|
|
||
| await putShop(shopId, payload); | ||
| navigate("/shop"); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하나의 의견입니다! 😅
기존에 작성해주신 코드는
비동기 요청과 관련된 변수들을 가까이 두시려는 의도로 보이는데요 🤔
에러 발생가능성이 높은 비동기 요청과
에러 발생 가능성이 없는 변수를 번갈아봐야 해서
조금 읽기가 어려울 수 있을 것 같아요.
| try { | |
| setIsSubmitting(true); | |
| let imageUrl = imagePreview || ""; | |
| if (imageFile) { | |
| const presignedURL = await postImage(imageFile.name); | |
| await putImage(presignedURL, imageFile); | |
| imageUrl = getPublicURL(presignedURL); | |
| } | |
| const payload = { | |
| name: form.name.trim(), | |
| category: form.category, | |
| address1: form.address1, | |
| address2: form.address2.trim(), | |
| originalHourlyPay: hourlyPay, | |
| description: form.description.trim(), | |
| imageUrl, | |
| }; | |
| await putShop(shopId, payload); | |
| navigate("/shop"); | |
| } finally { | |
| setIsSubmitting(false); | |
| } | |
| setIsSubmitting(true); | |
| let imageUrl = imagePreview || ""; | |
| const payload = { | |
| name: form.name.trim(), | |
| category: form.category, | |
| address1: form.address1, | |
| address2: form.address2.trim(), | |
| originalHourlyPay: hourlyPay, | |
| description: form.description.trim(), | |
| imageUrl, | |
| }; | |
| try { | |
| if (imageFile) { | |
| const presignedURL = await postImage(imageFile.name); | |
| await putImage(presignedURL, imageFile); | |
| imageUrl = getPublicURL(presignedURL); | |
| } | |
| await putShop(shopId, payload); | |
| navigate("/shop"); | |
| } finally { | |
| setIsSubmitting(false); | |
| } |
| <h2 className="sm:text-[1.75rem] text-[1.25rem] font-bold"> | ||
| 가게 정보 | ||
| </h2> | ||
| <button onClick={() => navigate("/shop")}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
라우트 상수를 활용하면 더 좋을 것 같아요 👍
import { ROUTES } from "./constants/router";
...
export default function ShopRegisterPage() {
...
return (
...
<button onClick={() => navigate(ROUTES.SHOP.ROOT)}>
...
);
}
jeonghwanJay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고 많으셨습니다 ~
| setForm((prev) => ({ ...prev, [key]: value })); | ||
| }; | ||
|
|
||
| const handleImageUpload = (e: ChangeEvent<HTMLInputElement>) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추후에 리팩토링을 하게 된다면 이미지 업로드 시 파일 크기나 형식 제한 정도 검증하는 유효성 검사를 고려해 볼 수 있을 것 같습니다.!
#️⃣연관된 이슈
📝 PR 유형
📝작업 내용
스크린샷 (선택)
💬리뷰 요구사항(선택)